Skip to content

chore: add Pyrameter test-shape enforcement#1358

Open
samsonasik wants to merge 6 commits into
codeigniter4:developfrom
samsonasik:chore-add-pyrameter
Open

chore: add Pyrameter test-shape enforcement#1358
samsonasik wants to merge 6 commits into
codeigniter4:developfrom
samsonasik:chore-add-pyrameter

Conversation

@samsonasik

@samsonasik samsonasik commented Jul 15, 2026

Copy link
Copy Markdown
Member

Description

This PR adds Pyrameter phpunit extension:

https://github.com/boundwize/pyrameter

Pyrameter classifies executed tests as unit, functional, integration, or e2e based on the code they use, then compares the totals with your target shape.

Per current tests, here the target limit defined:

Defines target limits:

  • Unit: minimum 40%
  • Functional: maximum 10%
  • Integration: maximum 50%
  • End-to-end: maximum 0%

and get output

=========
Pyrameter
=========

Shape:  Inverted Pyramid
Result: Passed ✓

        ▀▀▀▀▀▀▀▀▀▀▀▀▀  Integration  ✓
          ▀▀▀▀▀▀▀▀▀  Unit           ✓
            ▀▀▀▀▀  Functional       ✓
              ▼  E2E                ✓

+=============+=======+========+============+
|    KIND     | TESTS | ACTUAL |   TARGET   |
+=============+=======+========+============+
| Unit        |   277 |  44.8% | >= 40.0% ✓ |
+-------------+-------+--------+------------+
| Functional  |    38 |   6.2% | <= 10.0% ✓ |
+-------------+-------+--------+------------+
| Integration |   303 |  49.0% | <= 50.0% ✓ |
+-------------+-------+--------+------------+
| E2E         |     0 |   0.0% | <=  0.0% ✓ |
+-------------+-------+--------+------------+

Total: 618 tests

Your heavier tests outnumber your unit tests.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@samsonasik samsonasik changed the title chore(test): add Pyrameter test-shape enforcement chore: add Pyrameter test-shape enforcement Jul 15, 2026
@samsonasik
samsonasik marked this pull request as draft July 15, 2026 14:01
@samsonasik
samsonasik marked this pull request as ready for review July 15, 2026 14:07
@samsonasik

samsonasik commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Ready to review/merge 👍

@samsonasik

samsonasik commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

synced latest develop, I can rebase if needed 👍

@datamweb

datamweb commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

@samsonasik How were these thresholds determined, and what is the rationale behind choosing these specific percentages?

Could you please rebase the branch instead of using a merge commit? We'd prefer to keep the history linear.

@samsonasik
samsonasik force-pushed the chore-add-pyrameter branch from 5175689 to eb176ac Compare July 22, 2026 00:09
@samsonasik

samsonasik commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

@datamweb I've rebased it.

For classification is per class/function usage with allow excluding if some part is more weight than other, on this repo,

  • Load common usage example by PyrameterConfig::defaults()
  • make use of DatabaseTestCase will marked as integration test, unless use FeatureTestTrait is used.
  • Make use of FeatureTestTrait will be marked as functional test.
  • Otherwise, make them marked as unit test.

Ideally, unit test minimal should be the greater, and functional/integration will less, but on this repo use case, it seems functional/integration test is more than unit, so the pyramid is currently inverted.

I use existing condition as baseline, with unit minimal to 40%, but to ensure functional and integration test will not continue growing, I set maximum to them as 10% and 50%.

@michalsn

michalsn commented Jul 22, 2026

Copy link
Copy Markdown
Member

According to the described classification, tests using FeatureTestTrait should be functional. However, filter tests inherit this trait through AbstractFilterTestCase, and their concrete test files do not reference it directly.

Since Pyrameter scans the concrete test file, these tests appear to be classified as unit rather than functional.

Until the classification is reliable, I would prefer not to enable failOnViolation().

@samsonasik

samsonasik commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@michalsn sure, I've updated to mark AbstractFilterTestCase usage as functional test, and remove failOnViolation() usage.

Now, it already pyramid structure 👍

=========
Pyrameter
=========

Shape:  Healthy Pyramid
Result: Passed ✓

               ▲  E2E              ✓
             ▄▄▄▄▄  Functional     ✓
           ▄▄▄▄▄▄▄▄▄  Integration  ✓
         ▄▄▄▄▄▄▄▄▄▄▄▄▄  Unit       ✓

+=============+=======+========+============+
|    KIND     | TESTS | ACTUAL |   TARGET   |
+=============+=======+========+============+
| Unit        |   277 |  44.8% | >= 44.0% ✓ |
+-------------+-------+--------+------------+
| Functional  |    97 |  15.7% | <= 16.0% ✓ |
+-------------+-------+--------+------------+
| Integration |   244 |  39.5% | <= 40.0% ✓ |
+-------------+-------+--------+------------+
| E2E         |     0 |   0.0% | <=  0.0% ✓ |
+-------------+-------+--------+------------+

Total: 618 tests

Your test pyramid target passed.

michalsn
michalsn reviewed Jul 22, 2026
View reviewed changes

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samsonasik Thanks.

I think these limits are fitted too closely to our current values. Maybe we could start with something like: 40 / 20 / 50?

If we agree on the proposed thresholds, we should bring back failOnViolation(). Without it, this is report-only, and someone would need to notice any violation in the CI logs, so this PR would no longer enforce anything.

@samsonasik
samsonasik force-pushed the chore-add-pyrameter branch from 50a11b9 to 4c0977e Compare July 22, 2026 15:54
@samsonasik

samsonasik commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

@michalsn sure, I've applied:

  • set target to min:40 for unit test, max:20 for functional, and max:50 for integration test
  • enabled failOnViolation()

The result is now like below:

=========
Pyrameter
=========

Shape:  Healthy Pyramid
Result: Passed ✓

               ▲  E2E              ✓
             ▄▄▄▄▄  Functional     ✓
           ▄▄▄▄▄▄▄▄▄  Integration  ✓
         ▄▄▄▄▄▄▄▄▄▄▄▄▄  Unit       ✓

+=============+=======+========+============+
|    KIND     | TESTS | ACTUAL |   TARGET   |
+=============+=======+========+============+
| Unit        |   277 |  44.8% | >= 40.0% ✓ |
+-------------+-------+--------+------------+
| Functional  |    97 |  15.7% | <= 20.0% ✓ |
+-------------+-------+--------+------------+
| Integration |   244 |  39.5% | <= 50.0% ✓ |
+-------------+-------+--------+------------+
| E2E         |     0 |   0.0% | <=  0.0% ✓ |
+-------------+-------+--------+------------+

Total: 618 tests

Your test pyramid target passed.

@samsonasik
samsonasik requested a review from michalsn July 22, 2026 15:56
michalsn
michalsn approved these changes Jul 22, 2026
View reviewed changes

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Since this introduces a new dependency, I'd like to wait for at least one more approval before merging.

datamweb
datamweb approved these changes Jul 22, 2026
View reviewed changes

@datamweb datamweb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michalsn michalsn michalsn approved these changes
@datamweb datamweb datamweb approved these changes
@lonnieezell lonnieezell Awaiting requested review from lonnieezell
@kenjis kenjis Awaiting requested review from kenjis
@MGatner MGatner Awaiting requested review from MGatner
@paulbalandan paulbalandan Awaiting requested review from paulbalandan

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@samsonasik @datamweb @michalsn